fix(ui): align Repository & Self-Host typography and layout with Dashboard #477
Conversation
WalkthroughAdds muted and small typography to self-host repository list, repository cards, and page header; inserts a description block under the header, adjusts header container classes for responsive layout, refactors spacing and icon/text sizes, and removes the GithubRepositoriesSkeletonLoader export and implementation. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
view/app/self-host/page.tsx (1)
101-102: Verify semantic HTML structure (same concern as list-repositories.tsx).Similar to the concern raised in
list-repositories.tsx(line 69), passing aTypographyH1component to thelabelprop could create nested heading elements or multipleh1tags on the page. Please refer to the verification script in the previous file's review.
🧹 Nitpick comments (1)
view/app/self-host/components/github-repositories/list-repositories.tsx (1)
34-34: Redundant condition check.The
&& !isLoadingcheck is redundant since this code executes in the else block after theif (isLoading)check on line 30, meaning!isLoadingis always true here.Apply this diff to simplify:
- if (paginatedApplications?.length === 0 && !isLoading) { + if (paginatedApplications?.length === 0) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
view/app/self-host/components/github-repositories/list-repositories.tsx(3 hunks)view/app/self-host/components/github-repositories/repository-card.tsx(5 hunks)view/app/self-host/page.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
view/app/self-host/page.tsx (1)
view/components/ui/typography.tsx (1)
TypographyH1(1-7)
view/app/self-host/components/github-repositories/list-repositories.tsx (1)
view/components/ui/typography.tsx (1)
TypographyH1(1-7)
view/app/self-host/components/github-repositories/repository-card.tsx (1)
view/components/ui/typography.tsx (2)
TypographySmall(61-71)TypographyMuted(73-77)
🔇 Additional comments (8)
view/app/self-host/components/github-repositories/list-repositories.tsx (1)
9-10: LGTM! Typography import aligns with dashboard consistency.The import and comment clearly document the intent to match dashboard typography.
view/app/self-host/page.tsx (2)
19-20: LGTM! Typography imports consolidated.Consolidating the typography imports improves readability and maintainability.
83-85: LGTM! Consistent typography usage.The use of
TypographyH2andTypographyMutedmaintains consistency with the dashboard design system.view/app/self-host/components/github-repositories/repository-card.tsx (5)
9-10: LGTM! Typography imports added.The import statement and comment clearly indicate the use of dashboard typography components.
29-29: LGTM! Cleaned up double space in className.Removing the extra space between hover classes improves code consistency.
33-38: LGTM! Typography and sizing updates align with dashboard design.The changes correctly implement:
- Reduced icon size (20px) to match smaller typography
- Responsive text sizing (
text-sm sm:text-base)TypographySmallwrapper for consistent stylingThese changes align well with the PR objectives to match dashboard card design.
52-55: LGTM! Description wrapped with TypographyMuted.The
TypographyMutedwrapper provides consistent styling for secondary text across the application.
124-124: LGTM! Skeleton loader updated to match component structure.The skeleton CardTitle classes now match the updated component sizing (
text-sm sm:text-base), ensuring visual consistency during loading states.
view/app/self-host/components/github-repositories/list-repositories.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
view/app/self-host/page.tsx (1)
92-117: Refactor to use JSX children instead of children prop.The header changes correctly prevent nested headings and the description block aligns with the Dashboard pattern. However, the
childrenprop usage at line 101 should be refactored to use JSX children syntax instead.As per static analysis guidelines, apply this diff to use JSX children:
<DahboardUtilityHeader<Application> searchTerm={searchTerm} handleSearchChange={handleSearchChange} sortConfig={sortConfig} onSortChange={onSortChange} sortOptions={sortOptions} /* (changed) pass plain string, no nested heading */ label={t('selfHost.page.title')} className="mt-5 mb-2 flex flex-col sm:flex-row sm:justify-between sm:items-center gap-2" - children={ - <AnyPermissionGuard permissions={['deploy:create']} loadingFallback={null}> - <Button - className="mb-4 w-max flex justify-self-end mt-4" - onClick={() => { - router.push('/self-host/create'); - }} - > - {t('selfHost.page.createButton')} - </Button> - </AnyPermissionGuard> - } - /> + > + <AnyPermissionGuard permissions={['deploy:create']} loadingFallback={null}> + <Button + className="mb-4 w-max flex justify-self-end mt-4" + onClick={() => { + router.push('/self-host/create'); + }} + > + {t('selfHost.page.createButton')} + </Button> + </AnyPermissionGuard> + </DahboardUtilityHeader> {/* (added) description outside header */} <TypographyMuted className="mb-5"> {t('selfHost.page.description')}view/app/self-host/components/github-repositories/repository-card.tsx (1)
5-5: Remove or re-exportGithubRepositoriesSkeletonLoader.
Inlist-repositories.tsx(line 5) you importGithubRepositoriesSkeletonLoaderfromrepository-card.tsx, but that file no longer exports it—this will break the build. Either remove the import and its usage (line 31) or re-introduce and export the loader component inrepository-card.tsx.
🧹 Nitpick comments (2)
view/app/self-host/components/github-repositories/list-repositories.tsx (1)
34-40: Simplify the empty state wrapper.The
divwrapper withtext-centerclass may be redundant if the parent container already handles centering. Consider removing the wrapper unless explicit centering is required at this level.Apply this diff to simplify:
- if (paginatedApplications?.length === 0 && !isLoading) { - return ( - <div className="text-center"> - {t('selfHost.repositories.noRepositories')} - </div> - ); - } + if (paginatedApplications?.length === 0 && !isLoading) { + return t('selfHost.repositories.noRepositories'); + }view/app/self-host/components/github-repositories/repository-card.tsx (1)
33-38: Simplify to avoid redundant typography styling.The CardTitle already applies responsive text sizing (
text-xs sm:text-sm font-bold), making the nestedTypographySmallwrapper redundant.TypographySmallapplies its owntext-smandfont-mediumclasses, which may conflict with the CardTitle's explicit styling.Apply this diff to simplify:
- <CardTitle className="flex items-center gap-2 text-xs sm:text-sm font-bold"> + <CardTitle className="flex items-center gap-2"> <Github className="text-primary" size={20} /> <TypographySmall> {name || t('selfHost.repositoryCard.unnamed')} </TypographySmall>Or, if you want to keep the custom sizing on CardTitle, remove TypographySmall:
<CardTitle className="flex items-center gap-2 text-xs sm:text-sm font-bold"> <Github className="text-primary" size={20} /> - <TypographySmall> - {name || t('selfHost.repositoryCard.unnamed')} - </TypographySmall> + {name || t('selfHost.repositoryCard.unnamed')}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
view/app/self-host/components/github-repositories/list-repositories.tsx(3 hunks)view/app/self-host/components/github-repositories/repository-card.tsx(3 hunks)view/app/self-host/page.tsx(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
view/app/self-host/components/github-repositories/list-repositories.tsx (1)
view/components/ui/typography.tsx (1)
TypographyMuted(73-77)
view/app/self-host/components/github-repositories/repository-card.tsx (1)
view/components/ui/typography.tsx (2)
TypographySmall(61-71)TypographyMuted(73-77)
view/app/self-host/page.tsx (2)
view/components/rbac/PermissionGuard.tsx (2)
AnyPermissionGuard(42-58)ResourceGuard(78-95)view/components/ui/typography.tsx (1)
TypographyMuted(73-77)
🪛 Biome (2.1.2)
view/app/self-host/page.tsx
[error] 101-101: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
🔇 Additional comments (6)
view/app/self-host/components/github-repositories/list-repositories.tsx (2)
9-10: LGTM!The TypographyMuted import is necessary for the new description block added below.
72-79: Past issue resolved; layout and description align with Dashboard pattern.The changes correctly address the nested heading issue from the previous review by passing a plain string to the
labelprop instead of aTypographyH1component. The responsive flex layout and new description block maintain consistency with the Dashboard design.view/app/self-host/page.tsx (2)
19-20: LGTM!Consolidating the typography imports and adding TypographyMuted supports the new description blocks added throughout the file.
78-83: LGTM!The access denied message correctly uses TypographyH2 and TypographyMuted, maintaining consistency with the updated typography pattern across the application.
view/app/self-host/components/github-repositories/repository-card.tsx (2)
9-10: LGTM!The typography imports are necessary for the title and description changes that follow.
52-55: LGTM!Wrapping the description with TypographyMuted inside CardDescription correctly applies the muted styling while preserving the line-clamp behavior. This aligns with the consistent typography pattern across the application.
view/app/self-host/components/github-repositories/list-repositories.tsx
Outdated
Show resolved
Hide resolved
view/app/self-host/components/github-repositories/list-repositories.tsx
Outdated
Show resolved
Hide resolved
view/app/self-host/components/github-repositories/repository-card.tsx
Outdated
Show resolved
Hide resolved
view/app/self-host/components/github-repositories/repository-card.tsx
Outdated
Show resolved
Hide resolved
view/app/self-host/components/github-repositories/repository-card.tsx
Outdated
Show resolved
Hide resolved
|
@zhravan, Done |
There was a problem hiding this comment.
Overall LGTM, can you share the gif/image?
Use https://brie.io/ to capture the same.
2 participants
Changes Made
Self-Host (page.tsx):
Updated header to use TypographyH1 for title and TypographyMuted for description.
Unified layout spacing with flex flex-col sm:flex-row sm:justify-between sm:items-center gap-2.
Access denied message uses TypographyH2 + TypographyMuted for consistency.
Repositories List (list-repositories.tsx):
Header updated to TypographyH1 + TypographyMuted.
Layout aligned with Dashboard Utility Header.
Repository Card (repository-card.tsx):
Repository name updated to TypographySmall with text-xs sm:text-sm font-bold.
Description uses TypographyMuted for consistent style.
Alignment of badges, icons, and metadata (stars, forks, issues) unified with Dashboard card design.
Summary by CodeRabbit
Style
Documentation